- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 111
 
[Chat] Meilisearch message store #239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| 
           Wouldn't it be more related to Chat and the MessageStore?  | 
    
| 
           Ah yes, I wasn't sure about the interface to use, I'll check the store one.  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR title and body would need an update please
| 
           Yes, I need to finish the test in the same time, I'll push the updated version once it's done 🙂  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea, thanks! added some minor comments.
I wonder tho if we should move that feature into the store component - WDYT?
| 
           @chr-hertel I agree, the  Maybe we can move the classes/interfaces after this PR to prevent any bloating of it?  | 
    
          
 Agreed 👍  | 
    
| 
           What about a dedicated MessageStore component? or too much?  | 
    
| 
           @OskarStark Well, here's my humble opinion (I don't know how this package was initially designed and what are the "rules" that it must follow to comply with Symfony approach): The  Maybe we could find a new "way" to split the   | 
    
d4a2fba    to
    3ca29bc      
    Compare
  
    | 
           One thing I wonder is about having multiple conversations happening in parallel. Let's say per user. How would that work with this implementation? separate index?  | 
    
| 
           Actually, yes, multiple index. Another solution might be to use a "session" per agent, the session is initiated by the agent (constructor call, maybe an UUID?) then sent to the message store to "lock" messages per agent. I don't see a big technical bottleneck but IMO, this refactoring must be brought via another PR (I can work on it if you want).  | 
    
10e3653    to
    07a89f8      
    Compare
  
    07a89f8    to
    707dab5      
    Compare
  
    | 
           Hi @Guikingone, i guess this one got lost - still want to wrap this up?  | 
    
| 
           Hi @chr-hertel, yes, of course, shouldn't we wait until #675 is merged? This way we can directly move the class into the correct directory 🤔  | 
    
| 
           Needs a rebase now  | 
    
| 
           @OskarStark Yes, I'm facing a small issue during tests, might need a day or two to fix it and ensure that everything is working as expecting, I'll take a look at your suggestions ASAP.  | 
    
| 
           No worries  | 
    
2db78cb    to
    7a703d7      
    Compare
  
    d39829d    to
    6980a85      
    Compare
  
    6980a85    to
    8ae2176      
    Compare
  
    bca109f    to
    3a2cb46      
    Compare
  
    21209c9    to
    76e8224      
    Compare
  
    76e8224    to
    3303986      
    Compare
  
    3303986    to
    bbf483c      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 
           Thank you @Guikingone.  | 
    

Hi 👋🏻
Here's a small POC to store message bags outside of memory (for long term agents), the implementation is built around MS but the main interfaces are defined (Redis, Doctrine, etc, any bridge can use it) for any bridge (and heavily inspired by the
Storepackage).Let me know if the idea is interesting, if not, feel free to close the PR 🙂